Feat: userscripts subfolders #2674#2695
Feat: userscripts subfolders #2674#2695entropie-33 wants to merge 5 commits intominbrowser:masterfrom
Conversation
Using a feature of Node filesystem which reads every file recursievly. Also extract add method : - readScriptFileAt: Reonsposible for reading the content and push it to the list - loadScripts: Goes through script folder and compile every script
Since no script have the filename typology implemented, I came to the conclusion regarding actual documentation that there was an issue preventing scripts from loading. Extracted method to get filename without extension to improve readability. NOTE: Might be necessary to be more precise on the [documentation](https://github.com/minbrowser/min/wiki/userscripts)
|
In old versions of Min, userscripts worked by saving a file named ".js": https://github.com/minbrowser/min/wiki/userscripts/_compare/6fd447be081f8f9911ed8100d99b6361c7d8fb6a When we switched to the new format, we kept support for the old format so that existing userscripts would keep working, which is why the code allows that, even though it's not mentioned in the current docs. We could remove that support, but I don't see much reason to either; do you think it should be removed? |
|
Regarding the actual change, I think it would be good if (as the issue author suggests) the folders are mapped to submenus of the userscripts context menu, for scripts with the |
|
Thanks for your reply ! Interesting, my question was mainly raised by this bit of code where every script needs to be in the format seems to necessarily be www.website.js. My main concern about removing it was introducing a breaking change. For the issue itself, since the need wasn't clearly expressed for the UI part of having scripts/subfolders mapping I figure out it was not necessary since to have a look at the organization of your scripts you cloud simply click the button "see folder". Btw what would be the value to implement tampermonkey to benefit from their tool on this ? Also, I take the shot to ask you whenever you have considered adding tests and if there's a methodology for it ? I kinda like the projet and love the idea that I'm helping improving my browser which was the last not privacy oriented software in my stack. |
|
Nonetheless I'll open a new PR for script visualization since, in my opinion this one includes enough modification; from subfolder loading to refactor, I wouldn't have too much modification on this PR because the full functionality will require a lot more modifications than the ones here. |
|
Oh I think the confusion is maybe this part: https://github.com/minbrowser/min/blob/master/js/userscripts.js#L106 - "domain" isn't named very well, since it's actually just the file name. We first check for a header, which is similar to what you're suggesting (and what the current documentation describes): https://github.com/minbrowser/min/blob/master/js/userscripts.js#L115 Only if there is no header do we assume the filename is a domain: https://github.com/minbrowser/min/blob/master/js/userscripts.js#L124 You can try this out - if you write a script with any file name and a header matching what the current documentation shows, it should already work. Supporting Tampermonkey directly would be nice, but implementing Chrome extension support would require a lot more work. There are no automated tests currently, but you're welcome to add some if you'd like. |
|
Okidoki makes a lot more sense for the naming haha ! Then there's 3 cases :
Absolutely no problem with tests I wasn't trying to brag nor be condescendant, hope you didn't take it as criticism. Would it be that bad if we break legacy ? Anyway thanks for your reactivity ! |
|
Currently, it's:
I'd be OK with removing the legacy support if you think there's a reason to, but it also seems like it'd be fairly easy to keep the legacy support around. As long as that's the case I'm inclined to keep it.
It's fine, I know tests would be valuable; I've just never had the motivation to write them. |
No problemo ! I think the userscript base is good but needs a bit of refactoring in order to reflect clearly what it does. I'll work on this.
Sure, I'm not that comfy with Electron tho but would be a nice try. |
Context
Issue #2674 was requesting subfolders loading for scripts.
When taking a look at the
userscripts.jsfile I thought it would be good to :Interrogations
My refactors raise 1 main question regarding script loading :
Did I broke something from the way script loading was intended or was the code legacy ?